-
Notifications
You must be signed in to change notification settings - Fork 22
Regain control over the SDK libraries #1036
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
[SDK_LIBS_UPDATE] Updating static SDK libraries
| runs-on: ubuntu-latest | ||
| container: | ||
| image: ghcr.io/ledgerhq/ledger-app-builder/ledger-app-builder-lite:latest | ||
| strategy: | ||
| fail-fast: false | ||
| matrix: | ||
| target: | ||
| - core: cortex-m3 | ||
| se: st33 | ||
| - core: cortex-m35p+nodsp | ||
| se: st33k1 | ||
|
|
||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| # TODO: remove this next line before merging! | ||
| ref: feat/regain_control_over_libs | ||
| sparse-checkout: | | ||
| tools/build_clangrt_builtins.sh | ||
| sparse-checkout-cone-mode: false | ||
|
|
||
| - run: ./tools/build_clangrt_builtins.sh -t ${{ matrix.target.core }} -o artifact/arch/${{ matrix.target.se }}/lib | ||
|
|
||
| - uses: actions/upload-artifact@v4 | ||
| with: | ||
| name: arch-${{ matrix.target.se }} | ||
| path: artifact/ | ||
|
|
||
| merge: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we need to add a permissions block to the workflow. This block should specify the least privileges required for each job. For example:
- The
buildjob only needscontents: readto check out the repository. - The
mergejob may needcontents: readandactions: writefor artifact handling. - The
pr_createjob requirescontents: readandpull-requests: writeto create pull requests.
The permissions block can be added at the root level of the workflow to apply to all jobs or individually for each job. In this case, we will add permissions for each job to ensure granular control.
-
Copy modified lines R26-R27 -
Copy modified lines R58-R60 -
Copy modified lines R71-R73
| @@ -25,2 +25,4 @@ | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| container: | ||
| @@ -55,2 +57,5 @@ | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| actions: write | ||
| steps: | ||
| @@ -65,2 +70,5 @@ | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
| if: ${{ success() && inputs.create_pr }} |
| needs: build | ||
| runs-on: ubuntu-latest | ||
| steps: | ||
| - uses: actions/upload-artifact/merge@v4 | ||
| with: | ||
| name: arch | ||
| pattern: arch-* | ||
| delete-merged: true | ||
|
|
||
| pr_create: |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we will add a permissions block at the workflow level to define the minimal permissions required for the workflow. This block will apply to all jobs unless overridden at the job level. Based on the workflow's operations, the following permissions are required:
contents: readfor accessing the repository's contents.pull-requests: writefor creating pull requests in thepr_createjob.
We will add the permissions block at the top of the workflow file, just below the name field.
-
Copy modified lines R2-R4
| @@ -1,2 +1,5 @@ | ||
| name: Build clang runtime builtins | ||
| permissions: | ||
| contents: read | ||
| pull-requests: write | ||
|
|
| needs: merge | ||
| runs-on: ubuntu-latest | ||
| if: ${{ success() && inputs.create_pr }} | ||
| continue-on-error: true | ||
| steps: | ||
| - name: Clone repository | ||
| uses: actions/checkout@v4 | ||
| with: | ||
| # by default the action uses fetch-depth = 1, which creates | ||
| # shallow repositories from which we can't push | ||
| fetch-depth: 0 | ||
| ref: ${{ inputs.target_sdk_branch }} | ||
|
|
||
| - name: Download Binaries artifact | ||
| uses: actions/download-artifact@v4 | ||
| with: | ||
| name: arch | ||
|
|
||
| - name: PR creation | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| run: | | ||
| git config --global user.email ${{ env.GIT_USER_EMAIL }} | ||
| git config --global user.name ${{ env.GIT_USER_NAME }} | ||
| git switch --create ${{ env.UPDATE_BRANCH }} | ||
| git add -A . | ||
| git commit -m 'Updating static SDK libraries' | ||
| git push -u origin ${{ env.UPDATE_BRANCH }} | ||
| gh pr create -B ${{ inputs.target_sdk_branch }} --title '[SDK_LIBS_UPDATE] Updating static SDK libraries' --body 'Created by Github workflow "${{ github.workflow }}", job "${{ github.job }}", run "${{ github.run_id }}".' |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To fix the issue, we will add a permissions block to the pr_create job to explicitly define the minimal permissions required for its tasks. Specifically, the pr_create job needs contents: write to push changes to a branch and pull-requests: write to create a pull request. This change ensures that the GITHUB_TOKEN has only the necessary permissions, reducing the risk of unintended actions.
The permissions block will be added under the pr_create job definition in the .github/workflows/build_clangrt_builtins.yml file.
-
Copy modified lines R67-R69
| @@ -66,2 +66,5 @@ | ||
| if: ${{ success() && inputs.create_pr }} | ||
| permissions: | ||
| contents: write | ||
| pull-requests: write | ||
| continue-on-error: true |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1036 +/- ##
=======================================
Coverage 62.08% 62.08%
=======================================
Files 13 13
Lines 1817 1817
=======================================
Hits 1128 1128
Misses 689 689
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
cb002e9 to
bea58f1
Compare
bea58f1 to
0dd9032
Compare
|
Replaced by #1076 |
Based on @apaillier-ledger's work here: #924.
Corresponds to https://ledgerhq.atlassian.net/browse/B2CA-1995.
See https://github.com/LedgerHQ/ledger-secure-sdk/blob/feat/regain_control_over_libs/arch/libraries.md as documentation.
Also internal documentation:
libclang_rt.builtinsreplaceslibgccfor Nano X, Stax and Flex targets.github/workflows/build_clangrt_builtins.ymlworkflow and pushed through [SDK_LIBS_UPDATE] Updating static SDK libraries #1035libcandlibmremoved from the SDK and from now on the ones frompicolibcDebian package are usedledger-app-testerSYSROOTchange, most probably because of incorrect header inclusion)app-plugin-raribleapp-tron(fix example: Correct inclusion of used system header file app-tron#88)app-mimblewimblecoinapp-plugin-okxapp-acreapp-zcash-newDescription
Please provide a detailed description of what was done in this PR.
(And mention any links to an issue docs)
Changes include
Breaking changes
Please complete this section if any breaking changes have been made, otherwise delete it.
Additional comments
Please post additional comments in this section if you have them, otherwise delete it.